feat: add insert batch and streaming rate controls - #813
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jamesgao-jpg The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Replace environment and CloudInsert-specific batch sizing with a task-level insert batch size shared by CLI, REST, frontend, datasets, and runners. Keep streaming insert rate case-specific and validate its relationship to batching. Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
c4d3bd6 to
302f569
Compare
Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
Merging zilliztech#843 reintroduced config.NUM_PER_BATCH, which the insert-batch-size control feature removed. Route the AOSS serverless bulk chunk size through the task-level insert_batch_size instead, keeping the default of 100 and the non-positive guard. Signed-off-by: jamesgao-jpg <james.gao@zilliz.com>
XuanYang-cn
left a comment
XuanYang-cn
left a comment
There was a problem hiding this comment.
Please address the two inline findings before merging.
| self.required_doc_ids = {doc_id for qrels in self.gt_data for doc_id in qrels} | ||
| self.selected_doc_ids = self._build_selected_doc_ids() | ||
| is_filtered = filters is not None and filters.type != FilterOp.NonFilter | ||
| self.selected_doc_ids = self._build_selected_doc_ids() if is_filtered else None |
There was a problem hiding this comment.
For unfiltered capped FTS runs, setting selected_doc_ids to None makes the iterator load only the first data.size corpus documents while gt_data still contains every positive qrel. Any qrel document after the cap is therefore impossible to retrieve, which systematically lowers recall/NDCG/MRR. Please keep the qrel-preserving document selection for unfiltered runs too; omitting filter_id does not require bypassing _build_selected_doc_ids().
| inputType=InputType.Number, | ||
| inputConfig=dict(step=100, min=100, max=4_000, value=200), | ||
| inputHelp="fixed insertion rate (rows/s), must be divisible by 100", | ||
| inputConfig=dict(step=100, min=100, max=MAX_STREAMLIT_INT, value=500), |
There was a problem hiding this comment.
This hard-coded minimum rejects valid configurations. For example, insert_batch_size=10 and streaming_insert_rate=50 satisfy all documented constraints and are accepted by CLI/REST, but the frontend cannot represent them. Please use a minimum of 1 and let the existing batch/rate validation enforce the relationship.
3 participants
Summary
This upstream PR supersedes jamesgao-jpg/VectorDBBench#3.
insert_batch_sizeacross CLI, REST, frontend, datasets, runners, metrics, and load-reuse identitystreaming_insert_ratecase-specific and validate that it is positive, at least the batch size, and divisible by itNUM_PER_BATCHand the Cloud Insert-specific batch option with the common task setting--memorydb-pipeline-batch-size, while accepting the legacy serialized fieldCompatibility
--insert-batch-sizeand--streaming-insert-rate--cloud-insert-batch-sizecustom_case.batch_sizeand metricnum_per_batchvalues intoTaskConfig.insert_batch_sizeVerification
make lintmake unittestSigned-off-by: jamesgao-jpg james.gao@zilliz.com